Refactor AXFR code #92
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Refactored AXFR code to not use domainmetadata_change table anymore. Added new trigger and new trigger function on domainmetadata. Added TSIG key handling trough change and slavezone_change tables. Added GetChangedZonesBatchWithTSIG and GetChangedSlaveZonesWithTSIG. Added a new return type changedzonewithtsig. Other minor changes. Partially resolves PROD-2760 (1/2).
Improved GetChangedZonesBatchWithTSIG SQL performance by 18%. Amends PROD-2760.
| INNER JOIN nameserver ON nameserver_id = nameserver.id | ||
| INNER JOIN slavezone ON zone = slavezone.name LEFT JOIN domainmetadata ON slavezone.id = domainmetadata.domain_id AND domainmetadata.kind = 'slave' | ||
| WHERE nameserver.name = nameservername AND status = 'PENDING' | ||
| ORDER BY changetime ASC, slavezone_change.id ASC |
There was a problem hiding this comment.
After calling DeleteSlaveZone, the zone is removed from the slave zone table and there is an entry in the 'slavezone_change' table. However, this function doesn't return the change. This is probably because of INNER JOIN slavezone ON zone = slavezone.name, the slave zone doesn't exist.
| WHERE nameserver.name = nameservername AND status = 'PENDING' | ||
| GROUP BY zone | ||
| LIMIT changelimit) AS res | ||
| INNER JOIN zone ON res.zone = zone.name LEFT JOIN domainmetadata ON zone.id = domainmetadata.domain_id AND domainmetadata.kind = 'master' |
There was a problem hiding this comment.
Check the same as above.
Fixed join issue with GetChangedZonesBatchWithTSIG and GetChangedSlaveZonesWithTSIG functions where they would not fetch rows if zone was deleted. Amends PROD-2760.
server/schema/assigntsigkey.sql
Outdated
| @@ -1,12 +1,12 @@ | |||
| CREATE OR REPLACE FUNCTION AssignTSIGKey( | |||
| domain varchar, | |||
There was a problem hiding this comment.
@nemanja-zivkovic-atomia It is possible to assign the tsig key that doesn't exist from atomiadns. This is not a valid situation from HCP. I am not sure if this should be fixed?
There was a problem hiding this comment.
It is possible, only trough AtomiaDNS. It is not possible trough HCP.
@sasa-mladenovic-troxo I don't remember if we discussed this, but should we prevent this situation? We can do so by adding a foreign key in domainmetadata on tsigkey table.
There was a problem hiding this comment.
This seems to be allowed on PowerDNS as well. I'm fine to leave it like this for now.
Refactored bind syncer slave zones. Amends PROD-2760.
Renamed the property for tsig keys Amends PROD-2760.
jovana-stamenkovic-atomia
approved these changes
Dec 8, 2021
This issue can happen because we are still doing reload_updated_domainmetadata (for backward compatibility) and because we are not creating the table during clean install (as we shouldn't). reload_updated_domainmetadata calls GetChangedDomainIDs which reads from domainmetadata_change which may not exist.
# Conflicts: # server/debian/atomiadns-database.postinst # server/generate_wsdl.sh # server/schema/ddl.sql # server/schema/unassigntsigkey.sql # server/wsdl-atomiadns.wsdl # server/wsdl-type-schema.xsd
Update AXFR refactor to be backwards compatible and compatible with a minor release. Reverted AssignTSIGKey function to include kind parameter to enable backwards compatibility. Changed "use_tsig_keys" config to "disable_tsig_keys" in PowerDNS syncer and inverted the condition so that it doesn't disable TSIG keys by default. Changed "use_tsig_keys_for_slave_zones" to "disable_tsig_keys" in bind syncer to be consistent with PowerDNS syncer and so that it doesn't disable TSIG keys by default. Removed use_tsig_keys_for_slave_zones function as it's no longer needed. Improved exception handling of assign_tsig_key syncer function. Removed "use_tsig_keys" from the config file. Amends PROD-2760
Split functions GetChangedZonesBatchWithTSIG and GetChangedSlaveZonesWithTSIG to fetch zones/slavezones separate from TSIG keys. Amends PROD-2760
Split functions GetChangedZonesBatchWithTSIG and GetChangedSlaveZonesWithTSIG to fetch zones/slavezones separate from TSIG keys in bindsync. Amends PROD-2760
Renamed functions for handling tsig key assignment fetch: atomiaTSIGKeyList -> atomiaTSIGKeyAssignmentList atomiaTSIGKeyItem -> atomiaTSIGKeyAssignmentItem
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactored AXFR code to not use domainmetadata_change table anymore.
Added new trigger and new trigger function on domainmetadata.
Added TSIG key handling trough change and slavezone_change tables.
Added GetChangedZonesBatchWithTSIG and GetChangedSlaveZonesWithTSIG.
Added a new return type changedzonewithtsig.
Other minor changes.
Partially resolves PROD-2760 (1/2).